feat(ssr): make cloud components safe for SSR and hydration#431
Open
revaarathore11 wants to merge 2 commits intoelixir-cloud-aai:mainfrom
Open
feat(ssr): make cloud components safe for SSR and hydration#431revaarathore11 wants to merge 2 commits intoelixir-cloud-aai:mainfrom
revaarathore11 wants to merge 2 commits intoelixir-cloud-aai:mainfrom
Conversation
|
@revaarathore11 is attempting to deploy a commit to the elixir-cloud-aai Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
Reviewer's GuideAdds SSR-safe utilities and guards across the design and client packages so web components can be imported in server environments without crashing or causing hydration mismatches, including safe custom element registration, deterministic ID generation, and SSR documentation. Class diagram for new SSR utilities and updated componentsclassDiagram
class SsrUtils {
- idCounter : number
+ isBrowser() boolean
+ isServer() boolean
+ ssrSafeDefine(tagName string, constructor CustomElementConstructor) void
+ generateDeterministicId(prefix string) string
+ resetIdCounter() void
}
class EccUtilsDesignCollapsible {
- collapsibleId : string
+ open : boolean
+ disabled : boolean
}
class EccUtilsDesignMultiSelect {
- selectId : string
+ value : string[]
+ placeholder : string
+ disabled : boolean
}
class EccUtilsDesignSelect {
- selectId : string
+ value : string
+ disabled : boolean
}
class EccUtilsDesignCode {
+ setupThemeObserver() void
+ isDarkMode() boolean
}
SsrUtils <.. EccUtilsDesignCollapsible : uses generateDeterministicId
SsrUtils <.. EccUtilsDesignMultiSelect : uses generateDeterministicId
SsrUtils <.. EccUtilsDesignSelect : uses generateDeterministicId
SsrUtils <.. EccUtilsDesignCode : uses isBrowser
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- You’ve added an
ssrSafeDefinehelper but the variousindex.tsfiles for components still inline their owntypeof window !== 'undefined'guards; consider refactoring those to usessrSafeDefineso the SSR registration logic stays consistent and centralized. - The deterministic ID generator uses a single global counter across all component types; if you ever need prefix-local sequencing or more predictable IDs per component kind, it might be cleaner to keep per-prefix counters (e.g., a map of counters keyed by prefix) rather than sharing one global counter.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- You’ve added an `ssrSafeDefine` helper but the various `index.ts` files for components still inline their own `typeof window !== 'undefined'` guards; consider refactoring those to use `ssrSafeDefine` so the SSR registration logic stays consistent and centralized.
- The deterministic ID generator uses a single global counter across all component types; if you ever need prefix-local sequencing or more predictable IDs per component kind, it might be cleaner to keep per-prefix counters (e.g., a map of counters keyed by prefix) rather than sharing one global counter.
## Individual Comments
### Comment 1
<location> `packages/ecc-utils-design/src/components/code/code.ts:649-655` </location>
<code_context>
}
setupThemeObserver() {
+ // SSR guard: MutationObserver and document are not available on the server
+ if (!isBrowser()) return;
+
// Set up mutation observer to watch for theme changes
const observer = new MutationObserver(() => {
this.updateEditorTheme();
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Guard against environments where `MutationObserver` exists on `window` but is undefined or not supported.
In some constrained or older browser-like environments, `window` exists but `MutationObserver` does not, which would cause a runtime error when you instantiate it. Consider guarding its presence before use, e.g.:
```ts
if (!isBrowser() || typeof MutationObserver === "undefined") return;
```
before creating the observer.
```suggestion
setupThemeObserver() {
// Guard: MutationObserver and document are not available on the server or in some constrained environments
if (!isBrowser() || typeof MutationObserver === "undefined") return;
// Set up mutation observer to watch for theme changes
const observer = new MutationObserver(() => {
this.updateEditorTheme();
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Author
|
@uniqueg @anuragxxd please review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR adds Server-Side Rendering (SSR) safety to Cloud Components so they can be
imported and used in SSR environments (e.g. Next.js, Remix) without crashes or
hydration mismatches.
Web Components remain client-rendered, but this change ensures:
Fixes #418
What changed
-Added shared SSR utilities (
isBrowser,isServer,ssrSafeDefine) forenvironment detection and safe custom element registration
customElements.define()calls across packagesMath.random()IDs with deterministic generatorsdocs/SSR.md) with Next.js/Remix guidanceVerification
@elixir-cloud/designin Node.js does not crashNotes
This PR intentionally avoids framework-specific logic and keeps changes minimal
and reviewable while addressing the core SSR and hydration issues.
Summary by Sourcery
Add SSR-safe utilities and guards to make cloud web components safe to import in server-side environments while avoiding hydration issues.
New Features:
Enhancements:
Documentation: